fix(agent): enforce client identity for user-scope execution when broker is not SYSTEM - #1892
Conversation
The broker Windows executor's development (non-SYSTEM) mode ran commands under the broker process token regardless of which user connected to the pipe, so a user-scope install requested by a different user would land in the broker user's profile. Reject execution when the pipe client SID does not match the broker process user SID. Also clarify the SYSTEM-mode error when the target user has no active logon session, since user-scope and interactive operations require one. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Let maintainers know that an action is required on their side
|
|
Implementation notes (audit findings for DGW-436): Verified as already correct (SYSTEM/service mode) —
Gap fixed (non-SYSTEM mode) — the execution path is selected at runtime by checking whether the process token is LocalSystem, not by a cargo feature; a release binary launched as a regular user takes the same path. There, Why not a capability flag: Testing: new unit test |
The current-user execution path is selected at runtime whenever the broker does not run as SYSTEM, regardless of build profile. Rename comments, log messages, and the regression test accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Enforces client identity in non-SYSTEM agent execution to prevent operations from using the wrong Windows profile.
Changes:
- Rejects pipe clients whose SID differs from the broker user.
- Clarifies active-session errors and non-SYSTEM behavior.
- Adds mismatch coverage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let process_user_sid = token | ||
| .sid_and_attributes() | ||
| .context("failed to query current process token user SID")? | ||
| .sid; | ||
| if process_user_sid != ctx.user_sid { |
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
No findings.
The non-SYSTEM executor now compares the authenticated pipe client's captured SID with the broker process token SID before running the plan, and the added Windows test verifies mismatched identities are rejected. The protocol/API pass found no specification-defined discrepancy.
1fe893d
into
master
Ensures package operations always run in the requesting user's own context.
In service (SYSTEM) mode the broker already launches operations with the client user's real session token, so user-scope installs land in the correct profile (
%LOCALAPPDATA%, HKCU) and interactive installer UIs appear on the user's desktop rather than in session 0.This change closes a gap in the non-SYSTEM execution path, which is selected at runtime whenever the broker process does not run as SYSTEM (e.g. the agent binary launched manually as a regular user instead of as the Windows service — in any build profile, release included): the executor ran commands under the broker process token regardless of which user connected to the pipe, so a user-scope install requested by a different user would silently land in the broker user's profile. Such requests are now rejected with a clear error.
The error reported when the target user has no active logon session is also clarified, since user-scope and interactive operations require the user to be logged on.
Issue: DGW-436